Repository navigation
Approve RavenDB index definitions to catch changes that trigger a rebuild - #6013
ramonsmits wants to merge 2 commits into
Conversation
I'm still not sure this is enough
|
|
||
| foreach (var index in indexes.OrderBy(i => i.IndexName, StringComparer.Ordinal)) | ||
| { | ||
| Render(builder, index.CreateIndexDefinition()); |
There was a problem hiding this comment.
The rendered definitions don't match what ServiceControl deploys. Both document stores are created with SaveEnumsAsIntegers = true (EmbeddedDatabase.Connect and RavenExternalPersistenceLifecycle), and IndexCreation assigns the store conventions to each index before calling CreateIndexDefinition(). Here the indexes have the default conventions, so enum comparisons are rendered as strings:
Approved: docs.FailedMessages.Where(doc => doc.Status == "Archived")
Deployed: docs.FailedMessages.Where(doc => doc.Status == 4)
When I set the runtime conventions locally, the test in ServiceControl.Persistence.Tests.RavenDB fails on that line. The audit approved file only matches because the audit indexes have no enum literals.
Because of this, a change to the conventions, for example someone flipping SaveEnumsAsIntegers, changes every deployed definition and rebuilds every index, and this test does not notice.
| Render(builder, index.CreateIndexDefinition()); | |
| index.Conventions = new DocumentConventions { SaveEnumsAsIntegers = true }; | |
| Render(builder, index.CreateIndexDefinition()); |
(plus using Raven.Client.Documents.Conventions;, and the primary instance approved file needs re-approving)
Better still: move the conventions into one shared factory in ServiceControl.RavenDB that both lifecycles and this helper use, so they can't drift apart.
There was a problem hiding this comment.
Confirmed, and the audit file was affected too: Status in both message view maps renders as 4 : 3 with the deployed conventions instead of the enum names.
Done in 111d718 as the shared factory: DocumentStoreConventions.Create() in ServiceControl.RavenDB, used by EmbeddedDatabase.Connect, both RavenExternalPersistenceLifecycle classes and IndexDefinitionText. Both approved files are re-approved with the integer literals. A change to the conventions now changes the rendered definitions and fails the test.
|
Today, getting past this test only takes copying the 1. Record intent with each approved definition Keep a manifest next to the approved files with one entry per index: a hash of the rendered definition, the version in which it last changed, and the upgrade-guide link. For example: {
"MessagesViewIndex": { "hash": "…", "changedIn": "6.20.0", "upgradeGuide": "https://docs.particular.net/…" }
}The test fails if a hash changes but its entry is not updated. Re-approving then means stating which release changes the index and where operators can read about it. The manifest also gives release notes a reliable list of index changes. 2. Flag index changes on pull requests Add a workflow that triggers on PRs touching
Making that check required turns it into a gate rather than a reminder. 3. Check at release time In For the operator side, I opened #6015: warn at start-up, using |
03977d8 to
111d718
Compare
2 participants
A change to a RavenDB index definition makes RavenDB build a side-by-side replacement of the index at the next start-up. On a large audit database the rebuild takes days, and an index an operator locked after the Lucene migration does not receive the change until it is unlocked. 6.20.0 changed the audit
MessagesViewIndexandMessagesViewIndexWithFullTextSearchdefinitions without a note in the upgrade guide, so the rebuild surprised operators who unlocked the index on 6.21.2.This adds an approval test per RavenDB persister that renders the definition of every index the persister deploys. The rendered text contains the parts RavenDB compares when it decides to replace an index: maps, reduce, fields, configuration, additional sources and assemblies, output collection settings, lock mode, priority, state and deployment mode. The test discovers indexes the same way
IndexDeployment.CreateIndexesAsync(Assembly, ...)does, and runs without a RavenDB server.The document conventions are part of the deployed definition: with
SaveEnumsAsIntegersan enum literal in a map renders as a number. The conventions now come from one factory,DocumentStoreConventions.Create()inServiceControl.RavenDB, used by the embedded and external document stores and by the renderer, so the approved text is what the instances deploy and a change to the conventions fails the test.ServiceControl.RavenDB/DocumentStoreConventions.csis the shared factoryTestHelper/IndexDefinitionText.csrenders the definitions, shared by both test projectsIndexDefinitionApprovalsinServiceControl.Persistence.Tests.RavenDBandServiceControl.Audit.Persistence.Tests.RavenDB, with the approved files for the current definitionsdocs/testing.mdstates what to do when the test fails: record the changed indexes in the upgrade guide before approving the new definition